What do you plan to use the store for?

I'm not sure how viable selector-based reactivity would be for the charts as a general-purpose reactivity model, as the cost of a store.update() scales with the number of total selectors listening. If there are charts with large number of elements and each of those elements uses selectors, it could cause performance issues.

In the grid I've been wanting to explore event-based reactivity, maybe with something like functional lenses as they don't differ much from selectors in term of notation (selectors are the read aspect of lenses):

const pageLens = createLens('pagination.page')
// read
const currentPage = useGridSelector(pageLens)
// write
store.update(pageLens, page => page + 1) // next page

Alternatively, directly going for object paths as event names could be viable (I think):

// read
const currentPage = useGridSelector('pagination.page')
// write
store.update('pagination.page', page => page + 1)

What do you plan to use the store for?

The initial idea is to prevent useless rerender. In this specific test case, the tooltip and highlight update only if the mouse move from one band to another. Previously it was updated at each mouse move, and generated a rerender.

In the grid I've been wanting to explore event-based reactivity, maybe with something like functional lenses as they don't differ much from selectors in term of notation (selectors are the read aspect of lenses):

I'm not familiar at all with those concept. Fropm what I understand, the idea is to explicitly say which part of the object you are updating (pagination.page in your example) and then only trigger the listener that registered to either pagination or pagination.page

Current computational costs

With the current store implementation, we have 3 costs: the listener that run when the store updates, the selectors, and the react re-render.

So all the listner need to be called. But thanks to memorization each selector run only one. and render only if the selector result is a new one.

For example if we do store.update(prev => ({ ...prev, page: prev.page + 1 })) it should execute the following parts

Your proposal is to have an approach that does not even run the listener linked to selectHighlight.

Room for improvement

If charts have a lot of elements, it will be in the plotting part. For example the bars in a bar chart.

The chart state will contain small sub-states that impact all elements. For example the id and index to the highlighted item, the dimension of the chart, the hidden series, ...

The impact of those selectors will depend on how we use them. I tend to be in favor of solution 1 but might have some performances issue I do not anticipate

// First solution: 1 selector per series

function useBar() {
  const highlightedIdentifier = useSelect(selectHighlightState)
  const drawingArea = useSelect(selectDrawingArea)
  const xAxis = useSelect(selectXAxis)
  ...
  // Return an array of object defining the rectangles dimension and state.
}

function BarPlot() {
  const bars = useBars()

  return {bars.map(({ x, y, widht, height, isHighlighted, ...}) => <BarItem ... />)}
}

// Second solution: 1 selector per item
function BarItem({x, y, seriesId, dataIndex, ...}) {
  const highlightedIdentifier = useSelect(selectHighlightState)

  const isHighligthed = highlightedIdentifier.seriesId === seriesId && highlightedIdentifier.dataIndex === dataIndex

  return <rect x={x} classeName={[isHighligthed && 'MuiBarChart-highlighted']} />
}

function BarPlot() {
  const bars = useBars()

  return {bars.map(({ x, y, widht, height, ...}) => <BarItem ... />)}
}

I'm not familiar at all with those concept.

If you're familiar with jotai, lenses are read/write atoms, while selectors are read-only atoms. Lenses are basically just a getter/setter, though they're ideally created in a way that make it easy to specify the get/set parts in a single pass, e.g. in ramda.js it's lensPath(['pagination', 'page']).

For example if we do store.update(prev => ({ ...prev, page: prev.page + 1 })) it should execute the following parts

Yes exactly. The problem with global selector-based stores is that they basically have a single global event, "something changed". The goal would be to have multiple events, like "page changed" or "highlight changed". And the implementation would ideally skip having to create event names individually, possibly by using path-based lenses/selectors that would map directly to event names. The gist in the grid issue outlines the idea: #12405

The one big difference in usage would be that store updates would rather be written like store.update(pageLens, page => page + 1), because the store needs to know (efficiently) which is the path that changed to dispatch events to the places that are listening to that path.

I've done some monkey-patching in the grid to test the concept and it would save us a good amount of CPU time during scroll events due to the amount of listeners we have (around 15 x number_of_rendered_cells), I figure that could also have an impact for charts datapoints.

I tried to build a POC but I hit a snag around the event listener map, I think it should probably use something like a trie or maybe some other datastructure to keep listener retrieval fast. E.g. if ['usersById', 123, 'name] is modified, we need to retrieve the listeners for ['usersById'], ['usersById', 123] and ['usersById', 123, 'name']; and if ['usersById'] is modified, then we need to find all listeners that listen to ['usersById', (anything)].

With v8 preparation I'm a bit short on time to explore this further right now, but if you write your store/selectors like the grid does, it shouldn't be too painful to migrate later.

Thanks for the detailed explaination.

Since store/selector is already a big win, I propose to go with the current solution, and keep all the selectors as internals, such that we can move forward with this solution during the v8.alpha and keep the possibility to improve it with the event-based storage method once all breaking changes have been done

Is the goal to have a single store eventually? Or even a main store and fewer secondary stores?

If that is the case it would be a good idea to rename the file here, and move it to its own folder, as well as having more meaningful names for the context and the useStore as well.

Having everything related to these in a single folder would also help with discoverability. Right now we have files in internals/plugins/models, internals/plugins/utils, internals/plugins, hooks and context that are all kinda related to eachother.

should this prop come from useBarChartProps like before? (same for all other chart types)

Yeah, I had the same internal debate. My argument in favor of this behavior is that we explicitly show that this tootlip has noting special and is just a slots with default component.

The useBarChartProps is useful to dispatch the props objet between multiple components. But here it's fairly straight forward to propagate the slotProps?.tooltip

Sounds reasonable, though I don't think the intention is very clear in the code itself 😆

This and the slotprops below seem to be used in multiple places, should we have a specific type for them?

Yes, why not creating a ChartsTooltipSlots and ChartsTooltipSlotProps to avoid copy pasting.

The reason why I did it that was is that XxxxSlots is the typing of slots passed to the Xxxx component. But now slots.tooltip is not passed to the ChartsTooltip but to the SparklineChart

I think it would still be useful to have them. It would likely simplify our lives, and if anyone needs to know if/how they are actually passed to the ChartsTooltip they can just check that